Skip to content

fix: preserve snapshot digest through the client capture path — Phase 4#949

Merged
thymikee merged 2 commits into
feat/phase4-screenshot-digest-viewfrom
feat/phase4-snapshot-client-digest
Jun 30, 2026
Merged

fix: preserve snapshot digest through the client capture path — Phase 4#949
thymikee merged 2 commits into
feat/phase4-screenshot-digest-viewfrom
feat/phase4-snapshot-client-digest

Conversation

@thymikee

Copy link
Copy Markdown
Member

What

The follow-up I flagged on #945: client.capture.snapshot() has the identical digest-stripping gap as screenshot did. It always runs the daemon data through normalizeSnapshotResult (which expects the full nodes tree), so a non-default responseLevel digest ({ nodeCount, refs }, from the merged #942) collapsed to an empty snapshot before reaching SDK/CLI callers.

Same fix as #945's screenshot: the capture method is now level-aware — when the effective responseLevel (request override or client config) is non-default, it returns the leveled payload verbatim instead of normalizing. Default-level behavior is byte-identical. Return type stays CaptureSnapshotResult (a union cascades into every .nodes consumer; the caller opted into the level so the runtime value is the leveled shape).

Adds a shipped-path client test: capture.snapshot({ responseLevel: 'digest' }) returns the raw digest (nodeCount/refs preserved, no normalizer identifiers).

Stacked on #945 (it adds the isLeveledResponse helper); auto-retargets to main when #945 merges.

Verification

  • tsc --noEmit 0; oxfmt + oxlint --deny-warnings clean; fallow audit --base origin/main clean; vitest run src/__tests__/client.test.ts pass; Layering Guard empty.

@github-actions

github-actions Bot commented Jun 30, 2026

Copy link
Copy Markdown

Size Report

Metric Base Current Diff
JS raw 1.4 MB 1.4 MB +145 B
JS gzip 455.7 kB 455.8 kB +75 B
npm tarball 561.3 kB 561.4 kB +57 B
npm unpacked 2.0 MB 2.0 MB +145 B

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 27.2 ms 27.7 ms +0.5 ms
CLI --help 48.3 ms 49.0 ms +0.7 ms

Top changed chunks:

Chunk Raw diff Gzip diff
dist/src/generic.js +122 B +56 B
dist/src/9919.js +23 B +19 B

@thymikee

Copy link
Copy Markdown
Member Author

Blocking this for the shipped CLI path and sequencing.

The SDK/client side of the stack delta is right: client.capture.snapshot({ responseLevel: "digest" }) now returns the daemon digest verbatim instead of collapsing it through normalizeSnapshotResult().

But agent-device snapshot --level digest --json still does not preserve the digest payload. snapshot goes through the generic CLI path: runCliCommandWithOutput() calls formatCliOutput(), and the snapshot formatter still assumes the default CaptureSnapshotResult shape. It calls serializeSnapshotResult(result), which expects nodes and does not preserve digest-only nodeCount/refs, so the CLI JSON path drops the useful digest fields the same way screenshot did before the #945 follow-up.

Please add a CLI shipped-path test for snapshot --level digest --json and make non-default response levels bypass/default-proof the snapshot CLI formatter so the JSON output keeps nodeCount/refs.

Also, this PR is stacked on #945 (feat/phase4-screenshot-digest-view), which still has an open CLI digest blocker, so it should not be treated as independently merge-ready until that base is fixed or retargeted.

thymikee added 2 commits June 30, 2026 09:45
client.capture.snapshot() always ran the daemon data through
normalizeSnapshotResult, which expects the full `nodes` tree — so a non-default
responseLevel digest ({ nodeCount, refs }) collapsed to an empty snapshot, the
same gap #945 fixed for screenshot. Make it level-aware: when the effective
responseLevel is non-default, return the leveled payload verbatim. Default-level
behavior is unchanged. Adds a shipped-path client test asserting the digest
survives (nodeCount/refs preserved, no normalizer identifiers).
agent-device snapshot --level digest --json dropped nodeCount/refs: snapshot
goes through the generic CLI path (runCliCommandWithOutput -> formatCliOutput),
whose snapshot formatter serializes the default CaptureSnapshotResult shape and
discards digest-only fields. Make runGenericClientBackedCommand level-aware: for
a non-default responseLevel it emits the raw leveled payload verbatim (JSON /
JSON text), bypassing the default-shape formatter. This generalizes to any
generic-path command. Adds a snapshot CLI shipped-path test.
@thymikee

Copy link
Copy Markdown
Member Author

Fixed (pushed). agent-device snapshot --level digest --json now preserves nodeCount/refs: runGenericClientBackedCommand is level-aware and, for a non-default responseLevel, emits the raw leveled payload verbatim instead of the per-command formatter's default-shape serialization (serializeSnapshotResult, which expects nodes). This generalizes to any generic-path command, not just snapshot. Added the requested snapshot CLI shipped-path test.

Re sequencing: rebased this onto the updated #945 (which now has its CLI digest fix), so the stack is: #945 (screenshot client + CLI) → #949 (snapshot client + generic CLI). tsc/oxlint/fallow/layering clean; 1119 tests pass (the one blip was the known-flaky daemon-client mid-stream-abort test — green on re-run).

@thymikee thymikee force-pushed the feat/phase4-snapshot-client-digest branch from 028d1d0 to d11b0b2 Compare June 30, 2026 07:48
@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Jun 30, 2026
@thymikee

Copy link
Copy Markdown
Member Author

Reviewed the follow-up for the earlier CLI-path blocker.

The shipped path now preserves the digest: client.capture.snapshot({ responseLevel: "digest" }) bypasses the default snapshot normalizer, and runGenericClientBackedCommand() emits non-default response-level payloads from the raw command result instead of running them through the default-shape snapshot formatter. The added snapshot --level digest --json test covers the CLI path that was dropping nodeCount/refs.

Checks are green. I do not see remaining actionable blockers in this PR. Sequencing note: this is still stacked on #945, so it should land after #945 or be retargeted once that base is in main.

@thymikee thymikee merged commit 69ef3c3 into feat/phase4-screenshot-digest-view Jun 30, 2026
16 checks passed
@thymikee thymikee deleted the feat/phase4-snapshot-client-digest branch June 30, 2026 08:33
@github-actions

Copy link
Copy Markdown
PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-06-30 08:33 UTC

thymikee added a commit that referenced this pull request Jun 30, 2026
* feat: screenshot digest response-view — Phase 4

Add a `screenshot` entry to the Phase 4 RESPONSE_VIEWS registry. At
responseLevel `digest`, the view keeps the cheap result fields — the
captured `path` and the `artifacts` retrieval handle — and collapses the
token-heavy `overlayRefs` array (each carrying ref + label + three
geometry rects) to a total `overlayCount` plus the first 12 refs leveled
down to `{ ref, label }`. `default`/`full` return today's shape unchanged,
so unregistered and default-level responses stay byte-identical.

* fix: preserve the screenshot digest through the client capture path

The daemon screenshot view returns a leveled digest (path, overlayCount, leveled
overlayRefs, artifacts), but client.capture.screenshot() always ran the data
through readScreenshotResultData, which keeps only path + full-geometry overlay
refs and drops overlayCount/artifacts — so the digest never reached SDK/CLI
callers. Make the capture method level-aware: when the effective responseLevel
(request override or client config) is non-default, return the leveled payload
verbatim instead of normalizing it. Default-level behavior is unchanged.

Adds shipped-path client tests: capture.screenshot({ responseLevel: 'digest' })
returns the raw digest (overlayCount/artifacts preserved, no normalizer
identifiers); the default path still normalizes. Kept the return type as
CaptureScreenshotResult (the union variant cascaded into many default-path
consumers); the caller opted into the level, so the runtime value is leveled.

* fix: preserve the screenshot digest through the CLI command path

agent-device screenshot --level digest --json still dropped overlayCount and
artifacts: the CLI command rebuilt the default { path, overlayRefs } shape from
the (now leveled) client result. Make screenshotCommand level-aware — for a
non-default responseLevel it emits the leveled payload verbatim (JSON / JSON
text). Adds a CLI shipped-path test (--level digest --json preserves the digest;
the default level still emits the normalized shape). Factors the predicate into
a shared isNonDefaultResponseLevel in contracts.ts, reused by the client helper.

* fix: preserve snapshot digest through the client capture path — Phase 4 (#949)

* fix: preserve the snapshot digest through the client capture path

client.capture.snapshot() always ran the daemon data through
normalizeSnapshotResult, which expects the full `nodes` tree — so a non-default
responseLevel digest ({ nodeCount, refs }) collapsed to an empty snapshot, the
same gap #945 fixed for screenshot. Make it level-aware: when the effective
responseLevel is non-default, return the leveled payload verbatim. Default-level
behavior is unchanged. Adds a shipped-path client test asserting the digest
survives (nodeCount/refs preserved, no normalizer identifiers).

* fix: preserve leveled digests through the generic CLI path

agent-device snapshot --level digest --json dropped nodeCount/refs: snapshot
goes through the generic CLI path (runCliCommandWithOutput -> formatCliOutput),
whose snapshot formatter serializes the default CaptureSnapshotResult shape and
discards digest-only fields. Make runGenericClientBackedCommand level-aware: for
a non-default responseLevel it emits the raw leveled payload verbatim (JSON /
JSON text), bypassing the default-shape formatter. This generalizes to any
generic-path command. Adds a snapshot CLI shipped-path test.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-human Valid work that needs human implementation, judgment, or maintainer merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant